-
Notifications
You must be signed in to change notification settings - Fork 117
[Feature][config] per-tool auto-approve settings + nonInteractiveMode early-exit #146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
just saw it's a draft, sowwee :D |
still helpful! Thank You |
0da7dec to
c68e51f
Compare
|
I reimplement/rebased given the many changes since I last started this draft pr. |
Looks great! I have some time Boxing Day morning tomorrow to sort out outstanding pull requests so will take a look then 😄 Merry Christmas 🎄🎄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds configuration-based auto-approval for MCP server tools and nanocoder tools, along with improved non-interactive mode behavior. Users can configure which tools should automatically execute without requiring user confirmation through alwaysAllow configuration arrays.
Key changes:
- Added
alwaysAllowconfiguration field to MCP server configurations andnanocoderToolsconfiguration - Modified tool approval logic in nanocoder tools (
write_file,string_replace,execute_bash) and MCP client to checkalwaysAllowlists - Introduced non-interactive mode early-exit behavior when non-approved tools are encountered (though implementation has critical bugs)
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| source/wizard/validation.ts | Added validation for MCP server alwaysAllow field to ensure it's an array of strings |
| source/wizard/validation.spec.ts | Added tests for alwaysAllow validation (non-array and non-string items) |
| source/wizard/validation-array.spec.ts | Added test data with alwaysAllow examples |
| source/wizard/templates/mcp-templates.ts | Added alwaysAllow field to MCP server config interface |
| source/types/mcp.ts | Added alwaysAllow field with documentation to MCPServer type |
| source/types/config.ts | Added alwaysAllow and nanocoderTools.alwaysAllow fields to AppConfig |
| source/tools/write-file.tsx | Modified needsApproval to check nanocoder tools config |
| source/tools/string-replace.tsx | Modified needsApproval to check nanocoder tools config |
| source/tools/execute-bash.tsx | Modified needsApproval from boolean to function that checks config |
| source/mcp/mcp-client.ts | Added isToolAutoApproved method and integrated it into tool approval logic |
| source/mcp/mcp-client.spec.ts | Added tests for auto-approval behavior |
| source/hooks/chat-handler/conversation/conversation-loop.tsx | Added non-interactive mode allow-list logic (contains critical bugs) |
| source/config/nanocoder-tools-config.ts | New helper function to check if nanocoder tools are auto-approved |
| source/config/index.ts | Added loading of alwaysAllow and nanocoderTools configuration fields |
| source/commands/mcp.tsx | Added display of auto-approved tools in MCP command output |
| docs/mcp-configuration.md | Added documentation for alwaysAllow field |
| agents.config.example.json | Added example configurations with alwaysAllow fields |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Hey @namar0x0309 - we have done an awful lot of work on the main branch, I think we're ready to proceed with plans we spoke about before the big winter clean up including this :D |
… tools Add support for configuring automatic approval of specific MCP tools through the `alwaysAllow` property in server configurations. This allows certain trusted tools to execute without requiring user confirmation prompts. - Update configuration schema to include `alwaysAllow` arrays - Modify MCP client to respect auto-approval settings - Add documentation for the new configuration option - Include UI indication of auto-approved tools - Add test coverage for approval behavior
c68e51f to
fb17e1c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
will-lamerton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @namar0x0309 - looking great from my point of view! Couple points:
- Are you able to address the co-pilot review points and ensure all test files related to changes are updated?
- Are you able to document functionality for users in the README?
This is a great addition :D
Thanks! Tackling this today! |
…on and documentation Address all Copilot feedback from PR #146 regarding the alwaysAllow configuration feature for tool auto-approval. Changes: 1. Security improvements in example configuration: - Changed example from dangerous 'execute_bash' to safer 'write_file' - Added comprehensive security warning comment about auto-allowing powerful tools like shell execution 2. Enhanced validation for nanocoderTools.alwaysAllow: - Updated source/wizard/validation.ts to validate nanocoderTools.alwaysAllow is an array of strings - Added optional third parameter to validateConfig function - Added validation tests covering array validation, string entry validation, and valid configuration acceptance 3. Comprehensive test coverage for alwaysAllow functionality: - Added 7 new tests in source/tools/needs-approval.spec.ts - Tests verify tools in alwaysAllow skip approval in normal mode - Tests verify tools NOT in list still require approval - Tests cover multiple tools configuration and undefined/null handling - Implemented proper test lifecycle with before/afterEach/after hooks 4. Improved configuration documentation: - Added clear inline comments in agents.config.example.json distinguishing top-level alwaysAllow (non-interactive mode) vs nanocoderTools.alwaysAllow (all modes) - Clarified when to use each configuration type 5. Comprehensive README documentation: - Added new "Tool Auto-Approval Configuration" section - Documented both configuration types with clear explanations - Listed all available nanocoder tools - Included security warnings with specific tool risk examples - Provided real-world use case examples (read-only, developer, CI/CD) - Updated table of contents All tests passing (33/33 needs-approval tests, 25/25 validation tests).
Description
This PR adds configuration-based auto-approval for tools and improves non-interactive mode behavior. Users can now specify which MCP server tools and nanocoder tools should automatically run without requiring confirmation, streamlining trusted workflows while maintaining security.
Key Features:
Per-tool auto-approval via alwaysAllow configuration for both MCP servers and nanocoder tools
Non-interactive mode early-exit when tools requiring approval are encountered
Graceful error handling with clear messaging when approval would be required
Implementation Details
• Added alwaysAllow field to MCP server configuration schema (array of tool names)
• Added alwaysAllow field to nanocoderTools configuration (array of nanocoder tool names)
• Modified tool approval logic in conversation loop to check alwaysAllow lists before requiring confirmation
• Implemented non-interactive mode detection via --run flag
• Added early-exit behavior in non-interactive mode when non-approved tools are encountered
• Updated configuration validation to ensure alwaysAllow is an array of strings
• Configuration examples added to agents.config.example.json
Security Considerations
• Opt-in by design: No tools are auto-approved by default
• Users must explicitly list tool names in alwaysAllow arrays
• High-risk operations can be excluded from auto-approval lists
• Existing approval mechanisms remain unchanged for non-configured tools
• Clear documentation on security implications in MCP configuration guide
• Bash tool respects alwaysAllow configuration but users can choose to exclude it
Breaking Changes
• None - this is an additive feature that maintains backward compatibility
• All new configuration fields (alwaysAllow) are optional
• Existing configurations continue to work without modifications
Testing
• Added unit tests for alwaysAllow configuration validation
• Tests verify array validation (must be array of strings)
• Tests cover non-interactive mode early-exit scenarios
• Verified backward compatibility with existing configurations
• Manual testing with filesystem and GitHub MCP servers
• Tested both approved and non-approved tool scenarios
Documentation Updates
• Updated mcp-configuration.md with alwaysAllow field documentation
• Added configuration examples for common use cases
• Updated agents.config.example.json with example alwaysAllow configurations for MCP servers
• Added nanocoderTools.alwaysAllow example configuration
• Configuration validation updated to handle new optional fields
This feature significantly improves the developer experience for trusted and automated workflows (CI/CD, local development with trusted tools) while maintaining nanocoder's security-first approach.
Type of Change
Testing
Automated Tests
.spec.ts/tsxfilespnpm test:allcompletes successfully)Manual Testing
Checklist